Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lazy creation of the Set resolving #3329

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

rubenporras
Copy link
Contributor

By doing that, when resources already linked are loaded from storage or when no cyclic resolution takes place, no
memory needs to be allocated for the set.

@rubenporras
Copy link
Contributor Author

Also for #3317

For this PR, since the field is protected, subclasses can rely on the eager initialization of the field, even if cyclic resolution unlikely to be a common case.

I have however only found one case by searching through GitHub (https://github.com/folmate/refinery_class2Relational/blob/1a722b38e59aa07d21aa50eae2f4bcfb99e83709/subprojects/language/src/main/java/tools/refinery/language/resource/ProblemResource.java#L57C79-L57C100). Propose, that if this PR is merged, I create myself a PR for that project that fixes the problem if they would ever update Xtext.

By doing that, when resources already linked are loaded from storage or
when no cyclic resolution takes place, no
memory needs to be allocated for the set.

Signed-off-by: Rubén Porras Campo <[email protected]>
Copy link

github-actions bot commented Jan 30, 2025

Test Results

  6 461 files  ±0    6 461 suites  ±0   3h 11m 42s ⏱️ - 5m 18s
 43 241 tests ±0   42 657 ✅ ±0    584 💤 ±0  0 ❌ ±0 
170 051 runs  ±0  167 714 ✅ ±0  2 337 💤 ±0  0 ❌ ±0 

Results for commit 59b1c94. ± Comparison against base commit 2e919f4.

♻️ This comment has been updated with latest results.

@rubenporras
Copy link
Contributor Author

The error on MacOs on QuickDebugSourceInstallingCompilationParticipantTest looks unrelated to my change, is it maybe a flaky test?

I would like to rerun the job but I think I do not have the rights to do so. Could someone rerun it?

@LorenzoBettini
Copy link
Contributor

The error on MacOs on QuickDebugSourceInstallingCompilationParticipantTest looks unrelated to my change, is it maybe a flaky test?

I would like to rerun the job but I think I do not have the rights to do so. Could someone rerun it?

Indeed that's unrelated and it's a flaky test. I created a separate PR for that #3331

when it gets merged we can restart the failing job, though I don't think that's a blocker for this PR.
Let's wait for @szarnekow

@rubenporras
Copy link
Contributor Author

I was not totally sure it was unrelated, so I thought I would rather ask. Thanks for confirming it.

@LorenzoBettini
Copy link
Contributor

I'm closing and reopening to trigger rebuild against the new main branch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants